Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

manifest: DRY annotations and extensibility #340

Closed
wants to merge 1 commit into from

Conversation

wking
Copy link
Contributor

@wking wking commented Sep 23, 2016

As I suggested in #164. I've shifted the extensibility section to stand alone at the end of the file, and pushed the annotations meat into the lower-level manifest specification (linking there from the higher-level manifest-list specification).

The extensibility requirements and annotations field might arguably apply to our other JSON types as well, but I've left them off this commit to focus on making the current requirements more DRY without changing the specified behavior.

cc @jonboulle.

@jonboulle
Copy link
Contributor

jonboulle commented Sep 25, 2016

LGTM, thanks

Approved with PullApprove

@vbatts
Copy link
Member

vbatts commented Oct 6, 2016

please rebase

@wking
Copy link
Contributor Author

wking commented Oct 6, 2016 via email

@wking
Copy link
Contributor Author

wking commented Oct 6, 2016

Fixed extensibibility.myextensibility.md typo with d07ef3fe311e07.

@jonboulle
Copy link
Contributor

Ehh, I preferred it where it was - now worried about death by a thousand files (extensibility, canonicalization, etc etc). Can we combine them (into a generic "considerations" document or similar), or just put this back?

@wking
Copy link
Contributor Author

wking commented Oct 10, 2016

On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote:

Ehh, I preferred it where it was - now worried about death by a
thousand files (extensibility, canonicalization, etc etc). Can we
combine them (into a generic "considerations" document or similar)…

I doubt we'll have a thousand of these, and I prefer filenames that
are more clear than “considerations”. But I don't care that much, so
I can combine canonicalization and extensibility into a single
considerations.md later today.

As I suggested in the PR landing these blocks [1,2,3].  I've shifted
the extensibility section to a separate considerations.md, since it's
a generic policy that applies to both our manifest and manifest-list.
I've pushed the annotations meat into the lower-level manifest
specification (linking there from the higher-level manifest-list
specification).

The extensibility requirements and annotations field might arguably
apply to our other JSON types as well.  The separate extensibility
file sets the stage for that, but I've left the other types off this
commit to focus on making the current requirements more DRY without
changing the specified behavior.

My personal preference would be to have separate canonicalization.md
and extensibility.md files, but Jonathan wanted the single file:

On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote [4]:
> Ehh, I preferred it where it was - now worried about death by a
> thousand files (extensibility, canonicalization, etc etc). Can we
> combine them (into a generic "considerations" document or similar),
> or just put this back?

[1]: opencontainers#164 (comment)
[2]: opencontainers#164 (comment)
[3]: opencontainers#164 (comment)
[4]: opencontainers#340 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
Copy link
Contributor Author

wking commented Oct 11, 2016

Combined extensibility.md and canonicalization.md into a single considerations.md with e311e07e2a0d6e, although I liked it better as separate files.

@jonboulle
Copy link
Contributor

You could probably just come up with a better word than "considerations"
and make us both happy :-)

On 11 October 2016 at 17:03, W. Trevor King notifications@github.com
wrote:

Combined extensibility.md and canonicalization.md into a single
considerations.md with e311e07
e311e07
e2a0d6e
e2a0d6e,
although I liked it better as separate files
#340 (comment)
.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#340 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACewN05eVjMpmZzot-NOQDyPH09_OlSOks5qy6VRgaJpZM4KEmwS
.

@wking
Copy link
Contributor Author

wking commented Oct 11, 2016

On Tue, Oct 11, 2016 at 08:05:48AM -0700, Jonathan Boulle wrote:

You could probably just come up with a better word than
"considerations" and make us both happy :-)

These extensibility and canonicalization seem like completely
orthogonal ideas to me. The only thing tying them together is the
spec behind both is currently short ;).

@stevvooe
Copy link
Contributor

"A little duplication is better than the wrong abstraction"

  • Not sure who to attribute this to

@wking
Copy link
Contributor Author

wking commented Oct 18, 2016

On Tue, Oct 18, 2016 at 04:13:09PM -0700, Stephen Day wrote:

"A little duplication is better than the wrong abstraction"

How is the abstraction I'm proposing wrong? Maybe we can fix it
without duplication :p.

@stevvooe
Copy link
Contributor

@wking You're introducing another document that the reader has to switch to to get the proper context of understanding. It both distract and confuses.

@wking
Copy link
Contributor Author

wking commented Oct 19, 2016

On Tue, Oct 18, 2016 at 06:29:23PM -0700, Stephen Day wrote:

@wking You're introducing another document that the reader has to
switch to to get the proper context of understanding. It both
distract and confuses.

We already do a lot of linking to handle more basic details of the
spec. For example, #167 removed local descriptor duplication in favor
of links to descriptor.md. Many mediaType docs link to
media-types.md#compatibility-matrix. The platform docs link to Go's
docs for $GOOS and $GOARCH. I don't see much difference between those
and having the extensibility docs off in their own location.

Would you prefer this PR if I left the “Extensibility” sections alone
for now and limited the change to the manifest-list → manifest
annotations link? We can revist consolidating the extensibility
sections if/when we add similar extensibility rules to
application/vnd.oci.image.config.v1+json.

@stevvooe
Copy link
Contributor

@wking So, descriptor.md collected a common idea. It is reflected by a first-class object, even if it is embedded. This PR introduces some arbitrary "considerations" or "extensibility", which is some what meaningless.

It might be better to break out the annotations into its own document, as it describes a common sub-structure, similar to descriptor.

@wking
Copy link
Contributor Author

wking commented Oct 19, 2016

On Wed, Oct 19, 2016 at 11:19:45AM -0700, Stephen Day wrote:

It might be better to break out the annotations into its own
document, as it describes a common sub-structure, similar to
descriptor.

That makes a nice home for the annotations wording, but I think the
bit of logic without a good home is the extensibility wording. The
extensibility condition about arbitrary JSON properties beyond those
defined in the spec. I'd initially put it in an extensibility.md but
moved it into considerations.md to satisfy @jonboulle 1. Would you
put the extensibility wording in an annotations.md? It may be a
closer match with annotations (which is already an
extensibility-related location) than it is for canonicalization. But
it's still distinct from the annotations property itself.

@stevvooe
Copy link
Contributor

@wking Frankly, I'd rather have a little repetition here than have to bounce into the middle of other documents without context. Context matters. Though we may store it in our brain in such a manner, most people don't consume information as a graph of references.

@wking
Copy link
Contributor Author

wking commented Oct 19, 2016

On Wed, Oct 19, 2016 at 11:35:13AM -0700, Stephen Day wrote:

Frankly, I'd rather have a little repetition here than have to
bounce into the middle of other documents without context. Context
matters.

I agree that context is important. What sort of context are you
looking for for the extensibility constraint? We have slots on both
sides (e.g. in manifest.md where it links to the extensibility docs
and in the extensibility docs where it discusses the JSON media types
it covers). I'm happy to provide lots of context at both locations,
but I don't think duplicating the requirement itself for every JSON
media type that it applies to is the scaleable approach. The
duplicate approach is just redundant reading for anyone reading both
locations and redundant writing (hopefully ;) for anyone writing to
either location.

@stevvooe
Copy link
Contributor

@wking Please rebase this or let's close.

@wking
Copy link
Contributor Author

wking commented Jan 19, 2017 via email

@stevvooe
Copy link
Contributor

@wking Please resubmit after #501 lands.

@stevvooe stevvooe closed this Jan 20, 2017
wking added a commit to wking/image-spec that referenced this pull request Jan 24, 2017
As I suggested in the PR landing these blocks [1,2,3].  I've shifted
the extensibility section to a separate considerations.md, since it's
a generic policy that applies to both our manifest and manifest-list.

The extensibility requirements might arguably apply to our other JSON
types as well (like annotations, which were recently DRYed up in
f15a268, annotations: make a designated doc and DRY a bit, 2016-12-15,
opencontainers#501).  The new extensibility section sets the stage for that, but
I've left the other types off this commit to focus on making the
current requirements more DRY without changing the specified behavior.

My personal preference would be to have separate canonicalization.md
and extensibility.md files, but Jonathan wanted the single file:

On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote [4]:
> Ehh, I preferred it where it was - now worried about death by a
> thousand files (extensibility, canonicalization, etc etc). Can we
> combine them (into a generic "considerations" document or similar),
> or just put this back?

[1]: opencontainers#164 (comment)
[2]: opencontainers#164 (comment)
[3]: opencontainers#164 (comment)
[4]: opencontainers#340 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/image-spec that referenced this pull request Jan 24, 2017
As I suggested in the PR landing these blocks [1,2,3].  I've shifted
the extensibility section to a separate considerations.md, since it's
a generic policy that applies to both our manifest and manifest-list.

The extensibility requirements might arguably apply to our other JSON
types as well (like annotations, which were recently DRYed up in
f15a268, annotations: make a designated doc and DRY a bit, 2016-12-15,
opencontainers#501).  The new extensibility section sets the stage for that, but
I've left the other types off this commit to focus on making the
current requirements more DRY without changing the specified behavior.

My personal preference would be to have separate canonicalization.md
and extensibility.md files, but Jonathan wanted the single file:

On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote [4]:
> Ehh, I preferred it where it was - now worried about death by a
> thousand files (extensibility, canonicalization, etc etc). Can we
> combine them (into a generic "considerations" document or similar),
> or just put this back?

[1]: opencontainers#164 (comment)
[2]: opencontainers#164 (comment)
[3]: opencontainers#164 (comment)
[4]: opencontainers#340 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
dattgoswami9lk5g added a commit to dattgoswami9lk5g/bremlinr that referenced this pull request Jun 6, 2022
As I suggested in the PR landing these blocks [1,2,3].  I've shifted
the extensibility section to a separate considerations.md, since it's
a generic policy that applies to both our manifest and manifest-list.

The extensibility requirements might arguably apply to our other JSON
types as well (like annotations, which were recently DRYed up in
f15a268, annotations: make a designated doc and DRY a bit, 2016-12-15,
#501).  The new extensibility section sets the stage for that, but
I've left the other types off this commit to focus on making the
current requirements more DRY without changing the specified behavior.

My personal preference would be to have separate canonicalization.md
and extensibility.md files, but Jonathan wanted the single file:

On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote [4]:
> Ehh, I preferred it where it was - now worried about death by a
> thousand files (extensibility, canonicalization, etc etc). Can we
> combine them (into a generic "considerations" document or similar),
> or just put this back?

[1]: opencontainers/image-spec#164 (comment)
[2]: opencontainers/image-spec#164 (comment)
[3]: opencontainers/image-spec#164 (comment)
[4]: opencontainers/image-spec#340 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
7c00d pushed a commit to 7c00d/J1nHyeockKim that referenced this pull request Jun 6, 2022
As I suggested in the PR landing these blocks [1,2,3].  I've shifted
the extensibility section to a separate considerations.md, since it's
a generic policy that applies to both our manifest and manifest-list.

The extensibility requirements might arguably apply to our other JSON
types as well (like annotations, which were recently DRYed up in
f15a268, annotations: make a designated doc and DRY a bit, 2016-12-15,
#501).  The new extensibility section sets the stage for that, but
I've left the other types off this commit to focus on making the
current requirements more DRY without changing the specified behavior.

My personal preference would be to have separate canonicalization.md
and extensibility.md files, but Jonathan wanted the single file:

On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote [4]:
> Ehh, I preferred it where it was - now worried about death by a
> thousand files (extensibility, canonicalization, etc etc). Can we
> combine them (into a generic "considerations" document or similar),
> or just put this back?

[1]: opencontainers/image-spec#164 (comment)
[2]: opencontainers/image-spec#164 (comment)
[3]: opencontainers/image-spec#164 (comment)
[4]: opencontainers/image-spec#340 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
7c00d added a commit to 7c00d/J1nHyeockKim that referenced this pull request Jun 6, 2022
As I suggested in the PR landing these blocks [1,2,3].  I've shifted
the extensibility section to a separate considerations.md, since it's
a generic policy that applies to both our manifest and manifest-list.

The extensibility requirements might arguably apply to our other JSON
types as well (like annotations, which were recently DRYed up in
f15a268, annotations: make a designated doc and DRY a bit, 2016-12-15,
#501).  The new extensibility section sets the stage for that, but
I've left the other types off this commit to focus on making the
current requirements more DRY without changing the specified behavior.

My personal preference would be to have separate canonicalization.md
and extensibility.md files, but Jonathan wanted the single file:

On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote [4]:
> Ehh, I preferred it where it was - now worried about death by a
> thousand files (extensibility, canonicalization, etc etc). Can we
> combine them (into a generic "considerations" document or similar),
> or just put this back?

[1]: opencontainers/image-spec#164 (comment)
[2]: opencontainers/image-spec#164 (comment)
[3]: opencontainers/image-spec#164 (comment)
[4]: opencontainers/image-spec#340 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
laventuraw added a commit to laventuraw/Kihara-tony0 that referenced this pull request Jun 6, 2022
As I suggested in the PR landing these blocks [1,2,3].  I've shifted
the extensibility section to a separate considerations.md, since it's
a generic policy that applies to both our manifest and manifest-list.

The extensibility requirements might arguably apply to our other JSON
types as well (like annotations, which were recently DRYed up in
f15a268, annotations: make a designated doc and DRY a bit, 2016-12-15,
#501).  The new extensibility section sets the stage for that, but
I've left the other types off this commit to focus on making the
current requirements more DRY without changing the specified behavior.

My personal preference would be to have separate canonicalization.md
and extensibility.md files, but Jonathan wanted the single file:

On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote [4]:
> Ehh, I preferred it where it was - now worried about death by a
> thousand files (extensibility, canonicalization, etc etc). Can we
> combine them (into a generic "considerations" document or similar),
> or just put this back?

[1]: opencontainers/image-spec#164 (comment)
[2]: opencontainers/image-spec#164 (comment)
[3]: opencontainers/image-spec#164 (comment)
[4]: opencontainers/image-spec#340 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
tomalopbsr0tt added a commit to tomalopbsr0tt/fabiojosej that referenced this pull request Oct 6, 2022
As I suggested in the PR landing these blocks [1,2,3].  I've shifted
the extensibility section to a separate considerations.md, since it's
a generic policy that applies to both our manifest and manifest-list.

The extensibility requirements might arguably apply to our other JSON
types as well (like annotations, which were recently DRYed up in
f15a268, annotations: make a designated doc and DRY a bit, 2016-12-15,
#501).  The new extensibility section sets the stage for that, but
I've left the other types off this commit to focus on making the
current requirements more DRY without changing the specified behavior.

My personal preference would be to have separate canonicalization.md
and extensibility.md files, but Jonathan wanted the single file:

On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote [4]:
> Ehh, I preferred it where it was - now worried about death by a
> thousand files (extensibility, canonicalization, etc etc). Can we
> combine them (into a generic "considerations" document or similar),
> or just put this back?

[1]: opencontainers/image-spec#164 (comment)
[2]: opencontainers/image-spec#164 (comment)
[3]: opencontainers/image-spec#164 (comment)
[4]: opencontainers/image-spec#340 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants